feat(pairing): enter desktop codes on mobile - #8085

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 530c3a454e

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7ca304070

Verdict: REQUEST CHANGES

Reviewed base f0eb5575ffc9d5f57af4ed3f574529d997c83a0d through exact head e7ca30407067550dfafb3176f10ccf3d6cd939d1.

Blocking finding

P1 — Make a timed-out code submission recoverable without consuming an ambiguous extra guess.

Mobile publishes code-submit, waits 10 seconds, then clears the completer and request ID (mobile/lib/features/pairing/pairing_provider.dart:142-169). The desktop has already counted and recorded that submission before its proof/rejection is delivered (crates/buzz-core/src/pairing/session_desktop_code.rs:38-53). Therefore a delayed response crosses the local deadline after the remote state has changed:

• A late rejection no longer matches _codeRequestId, so it is recorded but ignored (pairing_provider.dart:545-559). Retrying consumes another remote attempt even though the user observed no result for the first one, making the five-guess lockout diverge from visible attempts.
• A correct submission advances desktop state and sends proof plus payload (desktop/src-tauri/src/commands/pairing.rs:440-451). If delivery is later than 10 seconds, mobile has no pending completer for the proof (pairing_provider.dart:606-609) and buffers the payload behind confirmation (pairing_provider.dart:655-663). The UI says “Couldn’t check the code. Try again” (mobile/lib/features/pairing/pairing_page/sas_verification_view.dart:63-83), but desktop no longer accepts another code submission. The session is wedged until its 120-second timeout.

Author action: make the deadline outcome reconcilable. A small safe option is to let the bounded session timeout own failure instead of using a shorter per-attempt timeout. If retry remains, retain one logical request and exact signed event ID, and make source replay return the cached outcome without charging another guess. A delayed correct proof must still leave code entry; a delayed terminal rejection must still surface lockout. Add deterministic tests for delayed wrong response, delayed correct proof+payload, and retry not incrementing the attempt budget twice.

Verification owner: author for deterministic core/mobile integration coverage; mobile release QA for interrupted-network behavior on device.

What held under review

• Capability negotiation is encrypted and exact-token gated; legacy clients remain on explicit desktop confirmation.
• The six-digit code is independently random and source-only. Signed peer identity, recipient tag, NIP-44 decryption, transcript proof, event deduplication, and the irreversible five-attempt core limit are coherently bound.
• The product/UI lane found no additional source defect in numeric entry, error semantics, biometrics choice, accessibility labeling, haptics/reduced motion, or completed-state reset.

Evidence and gaps

• Local clean detached HEAD and live PR head matched e7ca30407067550dfafb3176f10ccf3d6cd939d1; git diff --check passed.
• Exact-head GitHub Rust, Desktop, Mobile, Mobile Swift, Security, PostgreSQL, and integration domains are green. The current-range Codex Security Review job was cancelled and its status comment still says review required; repository security-review ownership should close that gate.
• Reviewer-local full cargo test -p buzz-core could not link because this host has not accepted the Xcode license. Full Flutter tests stopped in objective_c native-asset SDK discovery for the same host setup. These are confidence gaps, not additional author defects.
• Physical-device haptics, VoiceOver/TalkBack, and delayed cross-device delivery were not observed. Mobile release QA owns that residual verification.

The cryptographic boundary is in decent shape. The ten-second clock, unfortunately, has learned state mutation without recovery. Classic dungeon behavior.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: f0eb5575ffc9d5f57af4ed3f574529d997c83a0d..e7ca30407067550dfafb3176f10ccf3d6cd939d1 (exact live head e7ca30407067550dfafb3176f10ccf3d6cd939d1)

Risk: critical — pairing identity/proof, bounded guess authorization, cross-device state transitions, and failure recovery.

Blocking finding

P1 — make a timed-out code submission recoverable without consuming an ambiguous extra guess.

Mobile publishes code-submit, waits 10 seconds, then clears the completer and request ID (mobile/lib/features/pairing/pairing_provider.dart:142-169). Desktop counts and records that submission before its proof/rejection reaches mobile (crates/buzz-core/src/pairing/session_desktop_code.rs:38-53). A response delayed past the local deadline therefore crosses a mutated remote state:

• A late rejection no longer matches _codeRequestId, so it is recorded but ignored (pairing_provider.dart:545-559). Retrying consumes another remote attempt although the user observed no result for the first, making the five-guess lockout diverge from visible attempts.
• A correct submission advances desktop state and sends proof plus payload (desktop/src-tauri/src/commands/pairing.rs:440-451). If those arrive after 10 seconds, mobile has no pending completer for the proof (pairing_provider.dart:606-609) and buffers the payload behind confirmation (:655-663). The UI says “Couldn’t check the code. Try again” (pairing_page/sas_verification_view.dart:63-83), but desktop cannot accept another submission; pairing is wedged until the 120-second session timeout.

Author action: make the deadline outcome reconcilable. Safest small option: let the bounded session timeout own failure rather than a shorter attempt deadline. If retry remains, retain one logical request/exact signed event ID and make source replay return the cached outcome without charging another guess. A delayed correct proof must leave code entry; a delayed terminal rejection must surface lockout. Add deterministic delayed-wrong, delayed-correct-proof+payload, and retry-does-not-double-charge tests.

Verification owner: author for deterministic core/mobile integration coverage; mobile release QA for interrupted-network behavior on device.

Integrated review

Capability negotiation is encrypted and exact-token gated; legacy clients remain on explicit desktop confirmation. The six-digit code is independently random and source-only. Signed peer identity, recipient tag, NIP-44 decryption, transcript proof, event deduplication, and the irreversible five-attempt core limit are coherently bound. The product/UI lane found no additional defect after withdrawing an unproven interim sign-out concern; numeric entry, errors, biometrics choice, labels, haptics/reduced motion, and completed-state reset appear sound in the reviewed source/tests.

Validation and residual risk

• Clean detached HEAD and live head matched; git diff --check passed.
• Exact-head hosted Rust, Desktop, Mobile, Mobile Swift, Security, PostgreSQL, and integration domains are green.
• Current-range Codex Security Review is cancelled/statused “review required”; repository security-review ownership must close that external gate.
• Reviewer-local Rust/Flutter suites could not run because this host has not accepted the Xcode license and native SDK discovery fails. This is reviewer infrastructure, not another author defect.
• Physical-device haptics, screen readers, and delayed cross-device delivery remain release-QA verification gaps.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66edf50f81

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 66edf50f81bc75bd9ae07b06e4f0004d9e8f9d81 (base f0eb5575ffc9d5f57af4ed3f574529d997c83a0d).

The new commit changes only mobile/lib/features/channels/channels_page/community_switcher.dart; it does not touch the pairing state machine or its tests. Both independent lanes confirm the prior P1 remains, and systems review confirmed a second P2 already identified in the exact-head inline review.

P1 — reconcile timed-out submissions instead of charging outcome-ambiguous guesses

Mobile creates a fresh request/event, waits 10 seconds, then clears its completer and request correlation (mobile/lib/features/pairing/pairing_provider.dart:142-169,823-842). Desktop increments code_attempts and records the event before constructing the response (crates/buzz-core/src/pairing/session_desktop_code.rs:25-55); duplicate IDs error rather than replaying a cached result (session.rs:700-755).

A delayed wrong response is marked processed but ignored once _codeRequestId is null (pairing_provider.dart:545-559), so a retry consumes another remote guess and visible failures diverge from the five-guess budget. A delayed correct proof cannot resolve the cleared completer (:578-617), while the payload buffers behind confirmation (:655-663) after desktop has advanced and released its one payload (desktop/src-tauri/src/commands/pairing.rs:430-456). The UI first reports failure and asks for a retry that desktop can no longer accept.

Author action: remove the shorter attempt timeout and let the bounded session own failure, or preserve one logical/exact signed event and make source replay return a cached outcome without another charge. Add deterministic production-seam tests for delayed wrong, delayed correct proof+payload explicitly leaving code entry, and retry charging exactly once.

Verification owner: author for mobile/core/desktop integration tests and mutation proof; mobile release QA for interrupted-network/background behavior.

P2 — clear the extra desktop nsec copy on terminal code-entry success

start_pairing_session stores the Zeroizing<String> in PairingHandle.payload, clones it into PairingTaskContext, then clones it again into the worker (desktop/src-tauri/src/commands/pairing.rs:145-175,364-365). Code-entry success takes only the worker copy (:440-443). The handle copy is cleared only by start/cancel (:88-92,128,251-284), while successful UI completion deliberately skips cancel (desktop/src/features/settings/ui/MobilePairingCard.tsx:342-389). A completed pairing therefore retains the prepared nsec until another pairing/cancel or process exit.

Author action: transfer ownership instead of cloning, or clear the managed payload on every terminal worker path; add a regression proving successful code entry leaves no prepared secret.

Verification owner: author/native Desktop tests.

Exact-head local/remote equality, clean-worktree, git diff --check, just mobile-check, policy, and DCO checks passed. Local Rust/Flutter suites were blocked before execution by the reviewer host’s unaccepted Xcode license/native SDK discovery; physical-device interrupted delivery and screen-reader behavior were not observed. Those are confidence gaps, not additional defects. Several exact-head hosted Rust/Desktop/Mobile Swift/security jobs remained in progress at submission.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: f0eb5575ffc9d5f57af4ed3f574529d997c83a0d..66edf50f81bc75bd9ae07b06e4f0004d9e8f9d81 (exact live head 66edf50f81bc75bd9ae07b06e4f0004d9e8f9d81)

Risk: critical — pairing identity/proof, bounded authorization guesses, secret lifetime, and cross-device failure recovery.

The refreshed commit changes only mobile/lib/features/channels/channels_page/community_switcher.dart; no pairing timeout, correlation, replay, secret-lifetime, or delayed-response test changed. The prior timeout blocker therefore remains, and exact-head review found one additional secret-lifetime defect.

Blocking findings

P1 — A timed-out submission still consumes an outcome-ambiguous guess and can wedge a correct pairing.

Mobile publishes a fresh code-submit, waits 10 seconds, then clears its completer and request correlation (mobile/lib/features/pairing/pairing_provider.dart:142-169). Desktop increments code_attempts and records the signed event before its proof/rejection is delivered (crates/buzz-core/src/pairing/session_desktop_code.rs:25-55). A retry creates a new request and signed event rather than replaying an idempotent operation.

A late wrong response no longer matches _codeRequestId, so mobile records but ignores it (pairing_provider.dart:545-559); the user can reach the five-guess remote lockout after seeing fewer than five rejections. A late correct proof sets _sasConfirmReceived but cannot complete the cleared completer (:578-617), while the payload buffers behind the still-visible code-entry state (:655-663). The UI first says “Couldn’t check the code. Try again” (mobile/lib/features/pairing/pairing_page/sas_verification_view.dart:63-83), but desktop has advanced and cannot accept that retry (desktop/src-tauri/src/commands/pairing.rs:430-456). This contradicts the mobile contract that uncertain remote acceptance remain visibly pending and retries not duplicate an operation (VISION_MOBILE.md:10-13,50-51).

Existing tests deliver rejection/proof immediately (mobile/test/features/pairing/pairing_provider_test.dart:297-393); core duplicate coverage expects replay to fail rather than return a cached outcome (crates/buzz-core/src/pairing/session_code_entry_tests.rs:81-111).

Author action: remove the shorter outcome-ambiguous attempt deadline and let the bounded session timeout own failure, or preserve one exact logical request/event and replay a cached source outcome without another charge. Delayed terminal rejection must update visible lockout state; delayed correct proof plus payload must explicitly leave code entry. Add deterministic production-seam tests for delayed wrong, delayed correct proof+payload, and retry/no-double-charge, with mutation proof.

P2 — Successful code-entry pairing leaves an extra desktop nsec copy resident.

start_pairing_session stores the Zeroizing<String> payload in PairingHandle, clones it into PairingTaskContext, and the worker clones it again (desktop/src-tauri/src/commands/pairing.rs:145-175,364-365). Code-entry success consumes only the worker copy (:440-443). The handle copy is cleared only on a later start/cancel (:88-92,128,251-284), while the completed UI intentionally skips cancellation for done (desktop/src/features/settings/ui/MobilePairingCard.tsx:342-389). The exported private key consequently remains in memory until another pairing/cancel or process exit.

Author action: transfer ownership instead of cloning, or clear the managed payload on every terminal worker path. Add a native/Desktop regression asserting successful code entry leaves no prepared secret.

Verification owner: author for deterministic mobile/core/Desktop coverage; mobile release QA for interrupted-network/background-resume and screen-reader behavior on device.

Integrated review and validation

• Both protocol/state and product/UI adversarial review reproduced P1 from the exact-head state machine. No additional product/UI/accessibility defect was established in the refreshed community-switcher delta.
• Clean detached local HEAD and live PR head matched 66edf50f81bc75bd9ae07b06e4f0004d9e8f9d81; git status --porcelain was empty and git diff --check f0eb557..HEAD passed.
• just mobile-check passed at exact head. Reviewer-local just mobile-test and cargo test -p buzz-core could not complete because this host's Xcode license/native SDK discovery is unavailable. Those are reviewer-infrastructure confidence gaps, not author defects.
• At the final freshness poll, no hosted check had failed and 47 checks had passed; Desktop core, two Desktop integration shards, and Codex Security Review remained in progress. Those external gates retain their normal owners.
• Physical-device delayed delivery, background/resume, haptics, and VoiceOver/TalkBack were not observed and remain release-QA gaps.

The new coat of paint survived inspection. The pairing clock and spare key did not magically fix themselves underneath it.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: beed7cdeed

:bot: Jude’s code review agent — REQUEST CHANGES at exact head beed7cdeed5b27d645f3f640900528a73e7213b3 (base f0eb5575ffc9d5f57af4ed3f574529d997c83a0d).

Both previous blockers are fixed:

• Mobile now keeps one logical code request pending until protocol response or bounded session cleanup; concurrent retry does not publish another submission. Deterministic delayed tests hold responses beyond 11 seconds, prove one submission, surface delayed rejection, and move delayed proof+payload out of code entry.
• Desktop now shares one managed Arc<Mutex<Option<Zeroizing<String>>>>; successful code entry atomically takes the payload, terminal current-worker cleanup clears it, and generation fencing protects replacement sessions. Regressions assert consumption and stale-worker behavior.

P1 — the fifth allowed successful guess exceeds the production relay’s event cap

The production pair relay still allows only six attempted EVENTs per connection (crates/buzz-pair-relay/src/lib.rs:72-76,771-810), with an integration test requiring event seven to fail (crates/buzz-pair-relay/tests/integration.rs:608-657). A valid fifth-attempt success requires seven events on each side:

• source: challenge + four rejections + proof + identity payload;
• target: offer + five submissions + completion.

The source’s seventh event is rejected with session event limit reached, so the identity payload is not delivered. Desktop may still emit pairing-code-entered after its socket write while mobile waits without the secret. The core five-guess test does not traverse the relay, and mobile delayed tests use a controllable socket, so no production-seam test catches the contract mismatch.

Author action: raise or redesign the bounded per-connection (and corresponding delivery) budget to cover the complete worst-case protocol. Add a deterministic production-relay integration regression for four wrong guesses followed by a correct fifth through payload import and completion; mutation-prove restoring the six-event cap fails.

Verification owner: author for relay-backed regression and affected package gates; mobile release QA for physical interrupted-network/background behavior.

Exact-head local/remote equality, clean tree, formatting/analyze, diff, file-size, policy, and DCO evidence passed. Local Rust/Flutter execution remained reviewer-host Xcode-license/native-SDK blocked. Hosted unit failures included at least one reported unrelated flaky test, while other jobs were pending; those gate classifications are confidence/CI work and do not alter the source-proven relay defect.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head beed7cdeed5b27d645f3f640900528a73e7213b3 (live/local head reconfirmed; clean worktree).

The prior delayed-response/attempt-accounting and retained-desktop-nsec blockers are fixed: one logical mobile request remains pending without republishing, and desktop now consumes/clears one shared zeroizing payload with generation fencing. The loading-cover interaction/semantics issue is also fixed. Three author-actionable defects remain:

1. P1 — a valid fifth-attempt success exceeds the production pairing relay budget. crates/buzz-pair-relay/src/lib.rs:72-76,771-810 still permits only six attempted EVENTs per connection, and crates/buzz-pair-relay/tests/integration.rs:608-657 explicitly requires event seven to fail. Four wrong guesses then a correct fifth requires seven outbound events on each side: source challenge + four rejections + proof + payload; target offer + five submissions + completion. The source payload is therefore rejected with OK false: error: session event limit reached, while desktop/src-tauri/src/commands/pairing.rs:462-477 can still finish its socket writes and emit pairing-code-entered, leaving desktop claiming progress while mobile waits without the secret. Existing core/mobile tests bypass the real relay, so they cannot catch the incompatible budgets.

   Author action: align the bounded relay/delivery budgets with the complete worst-case protocol, and add a deterministic real-pair-relay regression proving four wrong guesses followed by a correct fifth delivers proof, payload, and completion. Restoring the six-event cap must make that test fail.

2. P2 — entering the desktop code before negotiation permanently blocks that code. In local-SAS mode, mobile/lib/features/pairing/pairing_page/sas_verification_view.dart:34,87-90 records the early desktop code as the rejected value. When requiresDesktopCode later supplies verifyDesktopCode, the same hook state survives because the widget key is only ValueKey(pairingState.sasCode) (mobile/lib/features/pairing/pairing_page.dart:189-194), and sas_verification_view.dart:54-61 refuses the same value without publishing it. Deleting and re-entering cannot recover.

   Author action: reset/re-evaluate local rejection state when remote desktop-code verification activates, and add a widget/notifier regression for code entry before the delayed desktop-code event followed by successful submission of that same code.

3. P2 — the QR can be displayed with materially less lifetime than the UI timeout. Desktop creates PairingSession before readiness (desktop/src-tauri/src/commands/pairing.rs:158-162); core starts its 120-second lifetime in new_source (crates/buzz-core/src/pairing/session.rs:50-51,123-145); readiness may then consume 35 seconds (pairing.rs:209-220). The worker’s separate 130-second timer starts only after readiness (pairing.rs:380-389). A QR can consequently become visible with about 85 seconds of protocol lifetime while the desktop UI remains active, and a late scan is silently rejected as expired.

   Author action: start/reset the protocol deadline when the QR becomes visible, or otherwise use one aligned lifetime across readiness, protocol validation, and UI; cover the maximum readiness delay plus a late valid scan deterministically.

Validation/gates: Hosted Rust unit testing failed in untouched buzz-agent::fake_llm::steer_rejected_on_run_id_mismatch after 627 passes (CI run 37337469960, job 111856390974); this is classified as a non-PR confidence gap, not an additional author defect. Other CI jobs were still pending at verdict time. Local focused Rust/Flutter execution was blocked by host Xcode-license / objective_c SDK tooling, also non-blocking reviewer infrastructure. Physical interrupted-network/background behavior remains unobserved.

Verification owners: author + CI for deterministic relay/core/mobile regressions and full required gates; :bot: Jude’s code review agent for refreshed exact-head review; mobile release QA for device-only interrupted-network/background behavior.

:bot: Jude’s code review agent

Verdict: APPROVE

Reviewed: exact live head ac0ad7c3004683e5db813492846d787404a2b475 (base 46e60206a47ed1f696d625df57a1e5dd2d019a42 at final poll).

The prior relay-budget blocker is fixed. Production now permits 8 attempted signed events per connection and 16 successful deliveries per recipient (crates/buzz-pair-relay/src/lib.rs:72-78,786-838). The worst case—four rejected guesses followed by a correct fifth—uses seven events in each direction, so proof, identity payload, and completion all fit while the relay remains bounded.

The new production-seam regression launches the real relay server and drives offer, challenge, four rejections, fifth-attempt proof, encrypted identity payload/decryption, imported-identity verification, and completion (crates/buzz-pair-relay/tests/desktop_code.rs:72-162). Restoring the old cap of six rejects source event seven—the payload—before delivery, causing the test’s accepted-ACK/import path to fail. Both independent lanes found that mutation causal and found no remaining author-actionable defect.

Previously fixed invariants remain intact:

• mobile retains one logical pending request until protocol response or bounded session cleanup, avoiding retry/double-charge (mobile/lib/features/pairing/pairing_provider.dart:142-174);
• desktop atomically consumes the shared zeroizing payload and clears terminal current-generation state (desktop/src-tauri/src/commands/pairing.rs:339-348,471-474), with stale-generation regression coverage (desktop/src-tauri/src/commands/pairing_generation_tests.rs:142-162);
• final-attempt rejection, successful completion, and interrupted-network states remain truthful and recoverable in the reviewed source/tests.

Author action: none.

Confidence gaps / verification owners: reviewer-local Rust and Flutter execution was blocked before tests by this host’s unaccepted Xcode license; the old-cap mutant was therefore source-verified but not independently executed. Reviewer/tooling owns that gap. Physical-device interrupted-network/background behavior remains release-QA verification. At the final exact-head poll, 17 checks passed, 17 were pending, 17 were skipped, and none had failed; CI retains ownership of terminal required gates.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac0ad7c300

:bot: Jude’s code review agent — APPROVE for exact head ac0ad7c3004683e5db813492846d787404a2b475.

No author-actionable defect remains after integrating the protocol/runtime and product/UI lanes.

• The production relay now permits eight signed EVENT attempts per connection and sixteen successful deliveries per recipient (crates/buzz-pair-relay/src/lib.rs:72-78,786-838). The worst-case valid flow uses seven per peer: source challenge + four rejections + proof + payload; target offer + five submissions + completion.
• crates/buzz-pair-relay/tests/desktop_code.rs:72-162 binds the regression to the production relay: two real WebSockets traverse four wrong guesses, the correct fifth, transcript proof, decrypted identity payload/import verification, and completion while requiring accepted ACKs and delivery for every event. Restoring the former six-event cap causally rejects source event seven (the payload), so forward() fails before import.
• Delayed responses still remain one logical mobile request without duplicate publication (mobile/lib/features/pairing/pairing_provider.dart:142-174). The shared zeroizing desktop payload is still consumed once and cleared only by the current generation (desktop/src-tauri/src/commands/pairing.rs:339-348,471-474; pairing_generation_tests.rs:142-162).
• Early code entry is re-evaluated when desktop-code negotiation activates, with notifier/widget coverage (sas_verification_view.dart:96-109; pairing_provider_test.dart:300-354). Protocol and desktop UI expiry now share one absolute session deadline (crates/buzz-core/src/pairing/session.rs:568-577; desktop/src-tauri/src/commands/pairing.rs:380-398).

Exact-head hosted gates completed without a failure; Codex Security Review was cancelled only after the PR merged. Local relay/mobile execution and executable old-cap mutation remained blocked by this review host’s unaccepted Xcode license / Objective-C SDK lookup. Those are reviewer confidence gaps, not author defects. Physical interrupted-network/background behavior remains release-QA-owned residual risk.